Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: allow wallet type from db to have preference #6245

Merged

Conversation

hansieodendaal
Copy link
Contributor

Description

  • Allow wallet type from the wallet database to have preference. This will allow old wallets without wallet-type fields to be accepted as software wallets when added to the config file.
  • Set newline_style = "Auto" as we do not use JavaScript cucumber anymore; this was an issue on Windows.

Motivation and Context

  • Wallets prior to the wallet-type PR could not be used.
  • Performing cargo fmt in Windows where the git preference has been set to check out as is and commit unix style forced CRLF. Auto will give preference to the actual line-endings.

How Has This Been Tested?

System-level testing

What process can a PR reviewer use to test or verify this change?

Breaking Changes

  • None
  • Requires data directory on base node to be deleted
  • Requires hard fork
  • Other - Please specify

- Allow wallet type from the wallet database to have preference. This will allow old wallets
without wallet type fields to be accepted as software wallets when added to the config file.
- Set `newline_style = "Auto"` as we do not use JavaScript cucumber anymore; this was an issue
  on Windows.
@hansieodendaal hansieodendaal requested a review from a team as a code owner April 2, 2024 12:58
Copy link

github-actions bot commented Apr 2, 2024

Test Results (CI)

    3 files    120 suites   38m 1s ⏱️
1 273 tests 1 273 ✅ 0 💤 0 ❌
3 811 runs  3 811 ✅ 0 💤 0 ❌

Results for commit a67f7ee.

@ghpbot-tari-project ghpbot-tari-project added P-acks_required Process - Requires more ACKs or utACKs P-reviews_required Process - Requires a review from a lead maintainer to be merged labels Apr 2, 2024
Copy link

github-actions bot commented Apr 2, 2024

Test Results (Integration tests)

 2 files  11 suites   56m 33s ⏱️
33 tests 31 ✅ 0 💤 2 ❌
36 runs  32 ✅ 0 💤 4 ❌

For more details on these failures, see this check.

Results for commit a67f7ee.

Copy link
Contributor

@brianp brianp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

utAck

Had to figure out why old wallets were a problem. I thought I had handled that situation. I've now realized I had initially made a change in the same function as you, but removed it and replaced it with a compile feature. Which means it didn't support old wallets when the feature was on. Which this would now fix.

@ghpbot-tari-project ghpbot-tari-project removed the P-reviews_required Process - Requires a review from a lead maintainer to be merged label Apr 2, 2024
@SWvheerden SWvheerden merged commit 70319cd into tari-project:development Apr 3, 2024
14 of 16 checks passed
@hansieodendaal hansieodendaal deleted the ho_allow_wallet_type branch April 3, 2024 08:48
sdbondi added a commit to sdbondi/tari that referenced this pull request Apr 15, 2024
* development:
  fix!: avoid `Encryptable` domain collisions (tari-project#6275)
  ci(fix): docker image build fix and ci improvements (tari-project#6270)
  feat: keep smt memory (tari-project#6265)
  feat: show warning when GRPC method is disallowed (tari-project#6246)
  fix(chat): metadata panic (tari-project#6247)
  feat: add monerod detection as an option to the merge mining proxy (tari-project#6248)
  chore(deps): bump h2 from 0.3.24 to 0.3.26 (tari-project#6250)
  feat: improve lmdb dynamic growth (tari-project#6242)
  feat: allow wallet type from db to have preference (tari-project#6245)
  feat: prevent mempool panic (tari-project#6239)
  ci: bump nightly version (tari-project#6241)
  feat: add validation for zero confirmation block sync (tari-project#6237)
  feat: new template with coinbase call (tari-project#6226)
  feat: improve wallet sql queries (tari-project#6232)
  chore: remove ahash as dependancy (tari-project#6238)
  feat: add dynamic growth to lmdb (tari-project#6231)
  chore(deps): bump borsh from 0.10.3 to 1.0.0 in /applications/minotari_ledger_wallet (tari-project#6236)
sdbondi added a commit to sdbondi/tari that referenced this pull request Apr 15, 2024
* development:
  fix!: avoid `Encryptable` domain collisions (tari-project#6275)
  ci(fix): docker image build fix and ci improvements (tari-project#6270)
  feat: keep smt memory (tari-project#6265)
  feat: show warning when GRPC method is disallowed (tari-project#6246)
  fix(chat): metadata panic (tari-project#6247)
  feat: add monerod detection as an option to the merge mining proxy (tari-project#6248)
  chore(deps): bump h2 from 0.3.24 to 0.3.26 (tari-project#6250)
  feat: improve lmdb dynamic growth (tari-project#6242)
  feat: allow wallet type from db to have preference (tari-project#6245)
  feat: prevent mempool panic (tari-project#6239)
  ci: bump nightly version (tari-project#6241)
  feat: add validation for zero confirmation block sync (tari-project#6237)
  feat: new template with coinbase call (tari-project#6226)
  feat: improve wallet sql queries (tari-project#6232)
  chore: remove ahash as dependancy (tari-project#6238)
  feat: add dynamic growth to lmdb (tari-project#6231)
  chore(deps): bump borsh from 0.10.3 to 1.0.0 in /applications/minotari_ledger_wallet (tari-project#6236)
stringhandler added a commit that referenced this pull request Apr 15, 2024
Motivation and Context
---
There was a bug in the wallet when using non-interactive that was fixed
in #6245

How Has This Been Tested?
---

What process can a PR reviewer use to test or verify this change?
---

<!-- Checklist -->
<!-- 1. Is the title of your PR in the form that would make nice release
notes? The title, excluding the conventional commit
tag, will be included exactly as is in the CHANGELOG, so please think
about it carefully. -->


Breaking Changes
---

- [x] None
- [ ] Requires data directory on base node to be deleted
- [ ] Requires hard fork
- [ ] Other - Please specify

<!-- Does this include a breaking change? If so, include this line as a
footer -->
<!-- BREAKING CHANGE: Description what the user should do, e.g. delete a
database, resync the chain -->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P-acks_required Process - Requires more ACKs or utACKs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants